Skip to content

fix(nodes): pydantic field type massaging improvements #7984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 13, 2025

Conversation

psychedelicious
Copy link
Collaborator

Summary

When we do our field type overrides to allow invocations to be instantiated without all required fields, we were not modifying the annotation of the field but did set the default value of the field to None.

This results in an error when doing a ser/de round trip. Here's what we end up doing:

from pydantic import BaseModel, Field

class MyModel(BaseModel):
    foo: str = Field(default=None)

And here is a simple round-trip, which should not error but which does:

MyModel(**MyModel().model_dump())
# ValidationError: 1 validation error for MyModel
# foo
#   Input should be a valid string [type=string_type, input_value=None, input_type=NoneType]
#     For further information visit https://errors.pydantic.dev/2.11/v/string_type

To fix this, we now check every incoming field and update its annotation to match its default value. In other words, when we override the default field value to None, we make its type annotation <original type> | None.

This prevents the error during deserialization.

This slightly alters the schema for all invocations and outputs - the values of all fields without default values are now typed as <original type> | None, reflecting the overrides.

This means the autogenerated types for fields have also changed for fields without defaults:

// Old
image?: components["schemas"]["ImageField"];

// New
image?: components["schemas"]["ImageField"] | null;

This does not break anything on the frontend.

Related Issues / Discussions

n/a

QA Instructions

  • Fuzz test workflows.
  • Testing on custom nodes is probably worthwhile, but thanks to our type gen system for core nodes, they all should be fine.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

@github-actions github-actions bot added python PRs that change python files invocations PRs that change invocations frontend PRs that change frontend files python-tests PRs that change python tests labels May 1, 2025
When we do our field type overrides to allow invocations to be instantiated without all required fields, we were not modifying the annotation of the field but did set the default value of the field to `None`.

This results in an error when doing a ser/de round trip. Here's what we end up doing:

```py
from pydantic import BaseModel, Field

class MyModel(BaseModel):
    foo: str = Field(default=None)
```

And here is a simple round-trip, which should not error but which does:

```py
MyModel(**MyModel().model_dump())
# ValidationError: 1 validation error for MyModel
# foo
#   Input should be a valid string [type=string_type, input_value=None, input_type=NoneType]
#     For further information visit https://errors.pydantic.dev/2.11/v/string_type
```

To fix this, we now check every incoming field and update its annotation to match its default value. In other words, when we override the default field value to `None`, we make its type annotation `<original type> | None`.

This prevents the error during deserialization.

This slightly alters the schema for all invocations and outputs - the values of all fields without default values are now typed as `<original type> | None`, reflecting the overrides.

This means the autogenerated types for fields have also changed for fields without defaults:

```ts
// Old
image?: components["schemas"]["ImageField"];

// New
image?: components["schemas"]["ImageField"] | null;
```

This does not break anything on the frontend.
This prevents issues where the node is defined with an invalid default value, which would guarantee an error during a ser/de roundtrip.

- Upstream issue requesting this functionality be built-in to pydantic: pydantic/pydantic#8722
- Upstream PR that implements the functionality: pydantic/pydantic-core#1593
@psychedelicious psychedelicious force-pushed the psyche/feat/pydantic-goat-offering branch from f043928 to d6f8c82 Compare May 13, 2025 04:32
@psychedelicious psychedelicious merged commit 1566e29 into main May 13, 2025
12 checks passed
@psychedelicious psychedelicious deleted the psyche/feat/pydantic-goat-offering branch May 13, 2025 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend PRs that change frontend files invocations PRs that change invocations python PRs that change python files python-tests PRs that change python tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants